Add support to run yul optimizer assembly test #15009

aarlt · 2024-04-11T22:33:02Z

This is needed in the context of ethdebug. I will need to write some tests on how optimiser steps are dealing with the debug attributes defined by #14857. The ideas is to implement different PRs per optimiser step on top of #14969, where this PR will enable the visualisation how these debug attributes got potentially merged - depending on the optimiser step.

For context, see e.g. #15087.

cameel

I see this is pretty much an exact copy of YulOptimizerTest.h at this point, so I assume it's unfinished. What is actually the idea behind this test? Just displaying the assembly next to the optimized Yul or something more?

test/libyul/YulOptimizerAssemblyTest.cpp

test/InteractiveTests.h

test/libyul/YulOptimizerAssemblyTest.cpp

ekpyron · 2024-05-13T14:27:51Z

I see this is pretty much an exact copy of YulOptimizerTest.h at this point, so I assume it's unfinished. What is actually the idea behind this test? Just displaying the assembly next to the optimized Yul or something more?

Just to answer this: the target here is to test the transport of the debugging context through any individual optimizer step and then to assembly (which is from where we'll output them in the end)

In any case, doesn't hurt to have, and not too much of a headache to just merge.

test/libyul/YulOptimizerAssemblyTest.cpp

cameel · 2024-05-16T17:27:58Z

test/libyul/YulOptimizerAssemblyTest.cpp

+	EVMObjectCompiler::compile(
+		*m_object,
+		adapter,
+		EVMDialect::strictAssemblyForEVMObjects(EVMVersion{}),


Why aren't we respecting the dialect and EVM version selected by the user here even though we do in the other places in this test case?

Same question for analyzeStrictAssertCorrect() above.

Also, it would be a good idea to have something EVM version-specific in the sample test case so that we can see if this works correctly.

test/libyul/yulOptimizerAssemblyTests/blockFlattener/basic.yul

test/libyul/YulOptimizerAssemblyTest.cpp

cameel · 2024-05-16T18:25:40Z

test/libyul/YulOptimizerAssemblyTest.cpp

+	m_object->analysisInfo = std::make_shared<AsmAnalysisInfo>(
+		AsmAnalyzer::analyzeStrictAssertCorrect(EVMDialect::strictAssemblyForEVMObjects(solidity::test::CommonOptions::get().evmVersion()), *m_object)
+	);


Is it really correct to simply overwrite the previous analysis info? Do we do that in real compilation too?

basically this is needed, because after the optimisation step the code might have slightly changed, so we need to update the analysis info. if this is not done, the evm code transform will fail.

cameel

This seems mostly ok now, except for the DebugAttributeCache thing - need to understand what that is about and why it's a part of this, seemingly unrelated, PR.

cameel · 2024-07-24T19:47:55Z

test/libyul/Common.h

 #include <vector>
 #include <memory>

+#include <libyul/AsmParser.h>


This should be above STL imports.

cameel · 2024-07-24T19:57:31Z

test/libyul/YulOptimizerAssemblyTest.cpp

+{
+	std::shared_ptr<Object> object;
+	std::shared_ptr<AsmAnalysisInfo> analysisInfo;
+	Parser::DebugAttributeCache::Ptr debugAttributeCache = std::make_shared<Parser::DebugAttributeCache>();


YulOptimizerTestCommon has m_debugAttributeCache. Shouldn't you be using it here instead?

cameel · 2024-07-24T20:02:52Z

test/libyul/YulOptimizerAssemblyTest.cpp

+		return TestResult::FatalError;
+	}
+
+	auto const printed = (object->subObjects.empty() ? AsmPrinter{ dialect }(*object->code) : object->toString(&dialect));


Suggested change

auto const printed = (object->subObjects.empty() ? AsmPrinter{ dialect }(*object->code) : object->toString(&dialect));

std::string const yulCode = (object->subObjects.empty() ? AsmPrinter{dialect}(*object->code) : object->toString(&dialect));

By the way, would be good to add a simple smoke test that triggers the other branch here to make sure it works as expected.

cameel · 2024-07-24T20:07:24Z

libyul/ObjectParser.h

@@ -67,6 +69,7 @@ class ObjectParser: public langutil::ParserBase
 	void addNamedSubObject(Object& _container, YulString _name, std::shared_ptr<ObjectNode> _subObject);

 	Dialect const& m_dialect;
+	Parser::DebugAttributeCache::Ptr m_debugAttributeCache;


Do these optimizer changes related to DebugAttributeCache really belong in a PR that's adding a new test type? Shouldn't they be a part of some other PR specific to debug info?

cameel mentioned this pull request Apr 12, 2024

Clean up tab-space soup #15026

Merged

cameel reviewed Apr 12, 2024

View reviewed changes

test/libyul/YulOptimizerAssemblyTest.cpp Outdated Show resolved Hide resolved

test/InteractiveTests.h Outdated Show resolved Hide resolved

cameel added the testing 🔨 label Apr 12, 2024

aarlt force-pushed the add_yul_optimizer_assembly_test branch from e953eca to 917cec1 Compare April 18, 2024 21:01

aarlt commented Apr 22, 2024

View reviewed changes

test/libyul/YulOptimizerAssemblyTest.cpp Outdated Show resolved Hide resolved

aarlt mentioned this pull request Apr 22, 2024

Remove wordSizeTransform left-over. #15047

Merged

aarlt force-pushed the add_yul_optimizer_assembly_test branch 4 times, most recently from 8a4d556 to de39abd Compare April 22, 2024 15:19

github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label May 7, 2024

ethereum deleted a comment from github-actions bot May 7, 2024

aarlt removed the stale The issue/PR was marked as stale because it has been open for too long. label May 7, 2024

aarlt force-pushed the add_yul_optimizer_assembly_test branch from de39abd to 9357780 Compare May 7, 2024 12:19

aarlt marked this pull request as ready for review May 8, 2024 16:07

aarlt requested a review from cameel May 8, 2024 16:08

This was referenced May 9, 2024

Transport debug data through block flattener #15087

Closed

Transport debug data through common subexpression eliminator #15088

Draft

ekpyron previously approved these changes May 13, 2024

View reviewed changes

cameel reviewed May 16, 2024

View reviewed changes

aarlt dismissed ekpyron’s stale review via 7f1e1f4 May 21, 2024 22:11

aarlt force-pushed the add_yul_optimizer_assembly_test branch 3 times, most recently from 8740a1b to b95b101 Compare May 21, 2024 22:26

aarlt requested a review from cameel May 21, 2024 22:33

github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Jun 5, 2024

cameel removed the stale The issue/PR was marked as stale because it has been open for too long. label Jun 6, 2024

github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Jun 20, 2024

cameel removed the stale The issue/PR was marked as stale because it has been open for too long. label Jun 20, 2024

ethereum deleted a comment from github-actions bot Jun 25, 2024

aarlt force-pushed the add_yul_optimizer_assembly_test branch 2 times, most recently from 7f16255 to b14086e Compare July 9, 2024 23:08

aarlt changed the base branch from develop to yul_transport_debugdata_attributes_to_assembly_items July 9, 2024 23:09

aarlt force-pushed the yul_transport_debugdata_attributes_to_assembly_items branch 7 times, most recently from f52ac21 to 9ad6a0d Compare July 11, 2024 05:49

[test] Add support to run yul optimizer assembly test.

d548cb1

aarlt force-pushed the add_yul_optimizer_assembly_test branch from b14086e to d548cb1 Compare July 11, 2024 06:37

aarlt changed the title ~~[test] Add support to run yul optimizer assembly test.~~ [ethdebug] Add support to run yul optimizer assembly test. Jul 11, 2024

aarlt added the has dependencies The PR depends on other PRs that must be merged first label Jul 17, 2024

aarlt self-assigned this Jul 17, 2024

cameel added the ethdebug label Jul 24, 2024

cameel changed the title ~~[ethdebug] Add support to run yul optimizer assembly test.~~ Add support to run yul optimizer assembly test Jul 24, 2024

ethereum deleted a comment from github-actions bot Jul 24, 2024

cameel reviewed Jul 24, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support to run yul optimizer assembly test #15009

Add support to run yul optimizer assembly test #15009

aarlt commented Apr 11, 2024 •

edited

Loading

cameel left a comment •

edited

Loading

ekpyron commented May 13, 2024

cameel May 16, 2024 •

edited

Loading

cameel May 16, 2024 •

edited

Loading

aarlt May 21, 2024

cameel left a comment •

edited

Loading

cameel Jul 24, 2024

cameel Jul 24, 2024

cameel Jul 24, 2024

cameel Jul 24, 2024

cameel Jul 24, 2024

	auto const printed = (object->subObjects.empty() ? AsmPrinter{ dialect }(*object->code) : object->toString(&dialect));
	std::string const yulCode = (object->subObjects.empty() ? AsmPrinter{dialect}(*object->code) : object->toString(&dialect));

Add support to run yul optimizer assembly test #15009

Are you sure you want to change the base?

Add support to run yul optimizer assembly test #15009

Conversation

aarlt commented Apr 11, 2024 • edited Loading

cameel left a comment • edited Loading

Choose a reason for hiding this comment

ekpyron commented May 13, 2024

cameel May 16, 2024 • edited Loading

Choose a reason for hiding this comment

cameel May 16, 2024 • edited Loading

Choose a reason for hiding this comment

aarlt May 21, 2024

Choose a reason for hiding this comment

cameel left a comment • edited Loading

Choose a reason for hiding this comment

cameel Jul 24, 2024

Choose a reason for hiding this comment

cameel Jul 24, 2024

Choose a reason for hiding this comment

cameel Jul 24, 2024

Choose a reason for hiding this comment

cameel Jul 24, 2024

Choose a reason for hiding this comment

cameel Jul 24, 2024

Choose a reason for hiding this comment

aarlt commented Apr 11, 2024 •

edited

Loading

cameel left a comment •

edited

Loading

cameel May 16, 2024 •

edited

Loading

cameel May 16, 2024 •

edited

Loading

cameel left a comment •

edited

Loading